Skip to content

Comments

Fix: Remove extraneous JSON preamble from LLM responses#10957

Closed
sto-j wants to merge 1 commit intonextcloud:mainfrom
sto-j:main
Closed

Fix: Remove extraneous JSON preamble from LLM responses#10957
sto-j wants to merge 1 commit intonextcloud:mainfrom
sto-j:main

Conversation

@sto-j
Copy link
Contributor

@sto-j sto-j commented Apr 2, 2025

The most of the LLM's return in JSON markdown format, example:
```json
{
"reply1": "Received, thanks...",
"reply2": "Thanks for forwarding this..."
}
```
Adding additional instructions didn't help.

This is solution to strip:
```json
```
from the answer.

The most of the LLM's return in JSON markdown format, example:
```json
{
  "reply1": "Received, thanks...",
  "reply2": "Thanks for forwarding this..."
}
```
Adding additional instructions did't help. 

This is solution to strip:
```json
```
from the answer. 

Signed-off-by: sto-j <162719726+sto-j@users.noreply.github.com>
@welcome
Copy link

welcome bot commented Apr 2, 2025

Thanks for opening your first pull request in this repository! ✌️

@ChristophWurst
Copy link
Member

Awesome, thanks a lot, @sto-j!!

@hamza221 could you please add unit test coverage for the existing code so we have something to extend for this change?

@hamza221
Copy link
Contributor

hamza221 commented Apr 3, 2025

Awesome, thanks a lot, @sto-j!!

@hamza221 could you please add unit test coverage for the existing code so we have something to extend for this change?

#10962

@ChristophWurst
Copy link
Member

@sto-j would you be interested in writing a test to cover the case?

If so, please update your branch, with a merge or a rebase, and add at least one test method where the LLM service returns a response with the specific preamble.

We can write the test as well. Just let us know what you prefer :)

@sto-j
Copy link
Contributor Author

sto-j commented Apr 4, 2025

@sto-j would you be interested in writing a test to cover the case?

If so, please update your branch, with a merge or a rebase, and add at least one test method where the LLM service returns a response with the specific preamble.

We can write the test as well. Just let us know what you prefer :)

@ChristophWurst I'm sorry, my schedule is full this month, the rest of this pull request is in your hands.
Please accept my apologies. Thank you for your understanding.
Regards

@ChristophWurst
Copy link
Member

No problem at all. We'll take over 🫡

Thanks a lot for the contributions :)

@ChristophWurst
Copy link
Member

Superseded by #10966

@github-actions
Copy link

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants